-
-
Notifications
You must be signed in to change notification settings - Fork 14.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix/clean up various ibus-engines #71442
Conversation
63b0cdc
to
4847d8a
Compare
Are these changes relevant to any other IBus input methods? |
Yes, all input methods should be checked and updated. I already did this in typing booster and some other in the past. Need to run now, will try to continue during the week. (Or anyone can feel free to push here.) Also did not test this other than by running the setup execs on non-ibus system. |
The installed tests seem to be failing at the moment:
I will need to check if adding |
91d1b1f
to
36b7a64
Compare
Apparently, fujiwarat/ibus-anthy@5a9e485 is not yet merged to a stable release, so no tests for ibus-anthy. Managed to get |
I manually tested all the engines and they seem to work except for uniemoji and hangul. Hopefully, the latest push fixes uniemoji (edit: it does) but hangul does not seem to print any errors so more debugging will be required. Also we should try to write some tests. |
The merging of the attrsets in f581284 is ugly but until we have NixOS/rfcs#58, there is not much room for improvement. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, any test case would help with keeping up on these packages 👍
I went commit by commit, here's what I noticed.
-
ibus-engines.libpinyin: 1.10.0 -> 1.11.1
Commit message needs indent fix. -
ibus-engines.anthy: clean up
I believe we should be using fetchFromGitHub for the fetch.
The commit msg has* 1.5.11 switched from intltool to anthy
. probably meant gettext? -
ibus-engines.uniemoji: modernize
Let's use placeholder -
nixos/ibus: install D-Bus services
Not sure if I'm remembering this correctly, but could we get issues with multiple uses of
programs.dconf.enable
in our modules?
👍 Yeah this RFC would make lots of code feel better. |
* https://github.com/libpinyin/ibus-libpinyin/releases/tag/1.10.91 * https://github.com/libpinyin/ibus-libpinyin/releases/tag/1.10.92 * https://github.com/libpinyin/ibus-libpinyin/releases/tag/1.11.0 * https://github.com/libpinyin/ibus-libpinyin/releases/tag/1.11.1 * intltool → gettext
* We are using python3.withPackages, which will be used via shebang. - gobject-introspection is used for setup hook
* format the expression and reorder to canonical order * gobject-introspection is used for setup hooks so it should be in native inputs * wrapPython is not necessary * 1.5.11 switched from intltool to gettext
* https://github.com/libhangul/ibus-hangul/releases/tag/1.5.2 * https://github.com/libhangul/ibus-hangul/releases/tag/1.5.3 * switch to gettext (libhangul/ibus-hangul@8745e3f) * drop wrapPython, patch shebangs hook will take care of that
* add setup again * patch shebangs instead of wrapping * format the expression
The nested list is intentional as it highlights important changes in each release. There are not many, though.
I keep confusing it too but
Oops, fixed.
👍
It should not matter unless someone sets it to something different than |
* Rely on patchShebangsHook instead of wrapPython * Format the expression
IBus contains some D-Bus services, we should install them too. And enable dconf properly.
For example, extra meta tags.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Favorite refactor was pkgs/data/misc/unicode-emoji/default.nix
and pkgs/tools/inputmethods/ibus/default.nix
.
Closes: #68837